remove any Reentrant locks in favor of a CAS linked list #241
Conversation
| } | ||
|
|
||
|
|
||
| @GuardedBy("lock") |
There was a problem hiding this comment.
You should fix up the @GuardedBy("lock") everywhere - its no longer true
| CompletableFuture[] queuedFuturesArray = new CompletableFuture[queueSize]; | ||
| Object[] callContextsArray = new Object[queueSize]; | ||
| int index = queueSize - 1; | ||
| while (loaderQueueEntryHead != null) { |
There was a problem hiding this comment.
You dont have t use an array and then asList it. Create the ArrayList and the use java.util.ArrayList#set to set a specific index
The ArrayList is an array under the covers - so tis the same. Actually the performance cost is the same as what you have here but the codes simpler
There was a problem hiding this comment.
Also you dont need to calculate queue size if you use ArrayList - but then again its defaults to size 10 - so maybe growing the array is more expensive over time then walking the list twice. We could keep a list size in a voilaitle int say
There was a problem hiding this comment.
I wanted to avoid any overhead that comes from arraylist.
Arraylist set throws oob exception and arraylist add with index does array copy
This PR removes any usage of Reentrant Lock in DataLoader in favor of using a self made Linked List and updates to it via AtomicReference CAS.
CacheMap change
This is a breaking change for the future cache
CacheMap.In order to avoid any locking
was replaced with
which is required to be atomic (like for example offered by ConcurrendHashMap)
This requirement also means that the underlying cache implementation must support this atomic operation.
Concurrent loads without batching, but with caching
Previously it was guaranteed that for one key with caching enabled and batching disabled, the dispatching only happens once.
Now because we avoid any form on lock this is not guaranteed anymore: there could be unnecessary dispatch calls when concurrent load calls are happening.
TODO: Update tests to avoid Collections.reverse
See https://github.com/graphql-java/java-dataloader/pull/241/files#diff-e44f0238fddb1d168a414ceef18d7194686b83ecf167cb794d2d03e714494c88R259